Skip to content

Add the preferKeepingExistingSuppression extension property to shunt removeUnused mode for non-standard checks#279

Open
kelvinou01 wants to merge 1 commit into
okelvin/fix-remove-unusedfrom
okelvin/keep-existing
Open

Add the preferKeepingExistingSuppression extension property to shunt removeUnused mode for non-standard checks#279
kelvinou01 wants to merge 1 commit into
okelvin/fix-remove-unusedfrom
okelvin/keep-existing

Conversation

@kelvinou01
Copy link
Copy Markdown
Contributor

@kelvinou01 kelvinou01 commented Dec 15, 2025

Before this PR

FLUP to #278

Our mental model of a BugChecker is

  • Uses the suppression mechanisms built into error-prone (e.g. not descending into the BugChecker at all, or using SuppressibleTreePathScanner)
  • A fix is always preferable to a suppression

Some checkers break this

  • RemoveUnused was overzealously removing instances of StrictUnusedVariable and NullAway. This is because they roll their own suppression checking, which don't respect -XoptIgnoreSuppressionAnnotations
  • RemoveUnused + Apply mode was autofixing suppressed instances of SafeLoggingPropagation by adding @DoNotLog and @Unsafe. However, these human-added suppressions are vetted and deliberate. The autofix downgrades the log-safety of the affected methods, which is undesireable

After this PR

Add the SuppressibleErrorProne:PreferKeepingExistingSuppression option, so removeUnused mode will shunt

  • Checks which behave in weird ways
  • Checks whose autofixes aren't desirable (should be the minority)

==COMMIT_MSG==
Add the preferKeepingExistingSuppression extension option to shunt removeUnused mode for non-standard checks
==COMMIT_MSG==

Possible downsides?

If we want to be extra safe, we can make removeUnused only remove rollout suppressions. This should be a small code change.

I am leaning towards preferring autofixes as the default. We should roll this out with a background excavator to discover more rogue/undesireable fixes before integrating this into our upgrade infrastructure.

@changelog-app
Copy link
Copy Markdown

changelog-app Bot commented Dec 15, 2025

Generate changelog in changelog/@unreleased

Type (Select exactly one)

  • Feature (Adding new functionality)
  • Improvement (Improving existing functionality)
  • Fix (Fixing an issue with existing functionality)
  • Break (Creating a new major version by breaking public APIs)
  • Deprecation (Removing functionality in a non-breaking way)
  • Migration (Automatically moving data/functionality to a new system)

Description

Add the SuppressibleErrorProne:PreferKeepingExistingSuppression extension option to shunt removeUnused mode for non-standard checks

Check the box to generate changelog(s)

  • Generate changelog entry

@kelvinou01 kelvinou01 changed the title Add the SuppressibleErrorProne:PreferKeepingExistingSuppression extension option to shunt removeUnused mode for non-standard checks Add the PreferKeepingExistingSuppression extension option to shunt removeUnused mode for non-standard checks Dec 15, 2025
@kelvinou01 kelvinou01 changed the title Add the PreferKeepingExistingSuppression extension option to shunt removeUnused mode for non-standard checks Add the preferKeepingExistingSuppression extension property to shunt removeUnused mode for non-standard checks Dec 15, 2025
@kelvinou01 kelvinou01 changed the base branch from develop to okelvin/fix-remove-unused December 15, 2025 11:57
@changelog-app
Copy link
Copy Markdown

changelog-app Bot commented Dec 15, 2025

Successfully generated changelog entry!

Need to regenerate?

Simply interact with the changelog bot comment again to regenerate these entries.


📋Changelog Preview

✨ Features

  • Add the SuppressibleErrorProne:PreferKeepingExistingSuppression extension option to shunt removeUnused mode for non-standard checks (#279)

@kelvinou01 kelvinou01 requested a review from CRogers December 15, 2025 18:33
@kelvinou01 kelvinou01 assigned aldexis and unassigned aldexis Dec 15, 2025
@kelvinou01 kelvinou01 requested a review from aldexis December 15, 2025 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants